Skip to content

keybindings: allow full-screen screenshot copying from menus - #13952

Closed
rumours86 wants to merge 1 commit into
linuxmint:masterfrom
rumours86:allow-screenshots-while-modal
Closed

rumours86 wants to merge 1 commit into
linuxmint:masterfrom
rumours86:allow-screenshots-while-modal

Conversation

@rumours86

@rumours86 rumours86 commented Aug 24, 2026 •

Copy link
Copy Markdown
Contributor

Proposed narrowed revision: commit 8b954a79d is pushed to the source branch. This closed PR currently still shows its original head/diff; the linked commit contains the one-line revision described below.

Allow Ctrl+Print to copy a full-screen screenshot while a menu or popup is open.

Change only MK.SCREENSHOT_CLIP from ActionMode.NORMAL to ActionMode.ALL, matching the existing full-screen MK.SCREENSHOT binding. Area and window screenshot bindings retain ActionMode.NORMAL.

This PR has been narrowed following review: starting an area picker closes the popup, and a window screenshot captures the last focused window, so those shortcuts do not address capturing an open menu.

Validation on Cinnamon/Muffin 6.7.8 master (Cinnamon 61c1b3d, Muffin 963a573), with automated XTEST input in an isolated X11 session: normal Ctrl+Print, menu Print and menu Ctrl+Print all pass. The clipboard image contains the open Cinnamon menu, verified against a screenshot reference and by visual inspection, and the same menu remains open after capture. The four area/window bindings are still NORMAL. This narrower change has not been verified using a physical keyboard or on Wayland.

Area/window/clipboard screenshot keys were registered with
ActionMode.NORMAL, so pressing e.g. <Shift>Print with any menu or applet
popup open did nothing - while the full-screen SCREENSHOT key (ALL) and
the volume keys worked fine in the same situation.  Capturing the screen
with a popup open is a legitimate and common request, and the pickers in
js/ui/screenshot.js handle running on top of an existing modal.

Register all screenshot media keys with ActionMode.ALL.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@mtwebster

Copy link
Copy Markdown
Member

Is this tested on muffin/cinnamon 6.7.*?

@rumours86

Copy link
Copy Markdown
Contributor Author

Live testing was on Cinnamon 6.6.9 (Mint 22.3), where the same behavior change was applied via the equivalent 6.6 code path (the Main.modalCount == 0 check in on_media_key_pressed): with a menu open, Shift+Print launches the area picker over the menu, Escape cancels cleanly, and a selection completes cleanly (the latter with #13948 applied — without it the picker's double-_ungrab bug can trigger regardless of how it was launched).

On 6.7/master I verified the path statically rather than live (I don't have a 6.7 environment running): the mode field from the MEDIA_KEYS table lands in entry.allowedModes, and both delivery paths — _filterKeybinding (WM) and the modal _stageEventHandler — share _shouldFilterKeybinding, which does the bitwise allowedModes & actionMode check. MK.SCREENSHOT is already ActionMode.ALL in that table and works while a menu is open, so this PR only brings the remaining five screenshot keys onto the same, already-exercised mechanism; no new code path is introduced.

Happy to adjust if you'd prefer this gated differently on 6.7.

🤖 Addressed by Claude Code

@mtwebster

Copy link
Copy Markdown
Member

I don't want speculative PRs - if you haven't tested this against git master, where it will be applied, then please don't open a pull request - this isn't a fix we'd backport to older versions.

What about your other, numerous PRs?

@mtwebster mtwebster closed this Aug 24, 2026
@rumours86

Copy link
Copy Markdown
Contributor Author

Fair enough — building current git master (muffin + cinnamon) right now to test against it properly; I'll report back here with results on master, and I'll do the same verification pass on the other PRs (#13948, #13949, #13950, #13951) before asking for any further review. For context on those: the patches were authored against master source, but live behavioral testing so far was on 6.6.9 — I'll close or update any of them that don't reproduce/hold on master.

🤖 Addressed by Claude Code

@rumours86

Copy link
Copy Markdown
Contributor Author

Done — I've now built current git master (muffin + cinnamon 6.7.5) and tested against it, nested with a scale-2 dummy monitor.

This PR, verified on master: with the patch, the AREA_SCREENSHOT entry registers with allowedModes = ActionMode.ALL; with a menu open (actionMode == POPUP) Main._shouldFilterKeybinding() returns false (allow) for it, while a NORMAL-mode key like LOGOUT still returns true (blocked) — so the change is precisely scoped. On unpatched master the same entry is blocked, reproducing the dead Shift+Print.

Status of the other PRs, all re-verified against master the same way:

Details posted on each PR. Apologies for the earlier speculative submission — everything still open is now tested where it applies.

🤖 Addressed by Claude Code

@rumours86

Copy link
Copy Markdown
Contributor Author

@mtwebster

@rumours86

Copy link
Copy Markdown
Contributor Author

Retested end to end against freshly built current master: Cinnamon 6.7.8 (61c1b3d1cb1d83e263507fd59aede221bce9abf8) and Muffin 6.7.8 (963a573eff827d740ba8ce9592f3b2873aa0b952), with current cinnamon-settings-daemon and cinnamon-screenshot.

This time the test injects actual X11 key events, runs the real media-key handler and screenshot client, and validates the resulting PNG or image/png clipboard data. It does not call _shouldFilterKeybinding() to simulate a key press.

Context Unpatched master This PR only
All six screenshot shortcuts, normal desktop 6/6 6/6
Print with a real menu open Works Works
The other five screenshot shortcuts with the menu open All blocked All work

All 12 patched cases pass. Area captures have the expected 200×140 geometry; repeating the area shortcut while its picker is active does not open another picker. After completion/closing the menu, a real click reaches the GTK test window. The #13952 run applies this PR alone, without #13948.

Environment: isolated Xvfb X11 session, real compositor/Clutter/St/Gio, private settings and session bus. A minimal private keyring-environment service returns an empty environment to the media-key daemon; no screenshot/keybinding handlers were stubbed or replaced. Wayland was not tested.

Could you reconsider/reopen this PR on the basis of these current-master results? The August report did build master, but its internal filtering probes were less complete than this input-to-image test.

I also revisited the related PRs: #13948 and #13949 now have native regression results and updated descriptions. I withdrew #13950 because I could not reproduce its reported rendering issue on current master. #13951 remains closed.

@mtwebster

Copy link
Copy Markdown
Member

This still is more or less useless, though -

  • Area screenshot - closes any menu/popup at the same time the overlay appears.
  • Window screenshot - only takes a screenshot of the most recently focused window, nothing to do with popups.

Did you test this with an actual keyboard?

The only one there's any point in allowing is the full screenshot-to-clipboard (MK.SCREENSHOT_CLIP).

I really don't see this as much of an issue either way - the new cinnamon-screenshot tool allows clipping in its ui, so quickly trimming to the area you want before saving is trivial now.

@rumours86 rumours86 changed the title keybindings: allow screenshot keys while a menu is open keybindings: allow full-screen screenshot copying from menus Sep 23, 2026
@rumours86

Copy link
Copy Markdown
Contributor Author

You're right about the area and window cases. The earlier 12/12 result checked successful image output, which was insufficient to establish the intended popup-capture behavior.

Those current-master tests used automated XTEST input in Xvfb. I have not verified this narrower change with a physical keyboard.

I narrowed the PR to a single line: only MK.SCREENSHOT_CLIP changes to ActionMode.ALL. All four area/window bindings remain NORMAL. The branch now contains one commit, 8b954a79d, based on master 61c1b3d. The closed PR still displays its original head/diff, so the linked commit is the narrowed revision.

The focused test now checks the actual clipboard image: Ctrl+Print captures the open Cinnamon menu and leaves that same menu open. The PNG visibly contains the menu, and an automated comparison against menu-open/menu-closed references confirms it. Normal Ctrl+Print and existing menu Print still work (3/3 checks; the baseline blocks menu Ctrl+Print).

Could you reconsider reopening this PR for that single-key change?

@mtwebster

Copy link
Copy Markdown
Member

That's fine

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants